feat(ecmascript): support dynamic tenary operator jsvalue #6452

kwonoj · 2023-11-14T23:54:32Z

Description

context: https://vercel.slack.com/archives/C03EWR7LGEN/p1699990767027949

Currently, if the input source have a non-statically analyzable tenary operator

await import(
              process.env.NEXT_RUNTIME === 'edge'
                ? 'next/dist/compiled/@vercel/og/index.edge.js'
                : 'next/dist/compiled/@vercel/og/index.node.js'
)

it becomes to the output like

(await __turbopack_require__("[project]/packages/next/dist/compiled/@vercel/og/index.edge.js [rsc] (ecmascript, loader)")(__turbopack_import__))

that takes first value of tenary regardless of the condition. In the analyze phase if tenary is dynamic it retuns JsValue::Alternatives, and it takes the first available value immediately.

PR changes its behavior by having new type of JsValue for tenary - if condExpr is not statically analyzable. I tried to use existing JsValue but that doesn't seem to work easily out of the box. However I may miss some obvious so PR might be refactored more simple way instead.

Closes PACK-1963

vercel · 2023-11-14T23:54:44Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
examples-nonmonorepo	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	Nov 17, 2023 0:21am
examples-vite-web	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	Nov 17, 2023 0:21am

9 Ignored Deployments

Name	Status	Preview	Updated (UTC)
examples-basic-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 17, 2023 0:21am
examples-cra-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 17, 2023 0:21am
examples-designsystem-docs	⬜️ Ignored (Inspect)	Visit Preview	Nov 17, 2023 0:21am
examples-gatsby-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 17, 2023 0:21am
examples-kitchensink-blog	⬜️ Ignored (Inspect)	Visit Preview	Nov 17, 2023 0:21am
examples-native-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 17, 2023 0:21am
examples-svelte-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 17, 2023 0:21am
examples-tailwind-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 17, 2023 0:21am
turbo-site	⬜️ Ignored (Inspect)	Visit Preview	Nov 17, 2023 0:21am

github-actions · 2023-11-14T23:59:48Z

✅ This change can build next-swc

github-actions · 2023-11-15T00:03:11Z

🟢 CI successful 🟢

Thanks

github-actions · 2023-11-15T00:11:57Z

Linux Benchmark for `e99af59`

Click to view benchmark

Test	Base	PR	%
bench_hmr_to_commit/Turbopack CSR/1000 modules	24.83ms ± 0.72ms	24.15ms ± 0.99ms	-2.71%
bench_hmr_to_eval/Turbopack CSR/1000 modules	23.44ms ± 0.86ms	23.85ms ± 0.72ms	+1.74%
bench_startup/Turbopack CSR/1000 modules	1255.24ms ± 5.50ms	1239.51ms ± 19.46ms	-1.25%

github-actions · 2023-11-15T00:14:57Z

⚠️ Turbopack Benchmark CI failed ⚠️

The following steps have failed in CI:

Turbopack Rust benchmark tests (linux)
Turbopack Rust benchmark tests (mac/win, non-blocking)

See workflow summary for details

jridgewell · 2023-11-15T01:12:30Z

crates/turbopack-ecmascript/src/analyzer/builtin.rs

+            if test.is_truthy() == Some(true) {
+                *value = take(cons);
+                true
+            } else if test.is_falsy() == Some(true) {


This is just test.is_truty() == Some(false), right? We can this into a branch above and save some cycles.

I think so, this is borrowing existing references from elsewhere (I recall it's binary operation or something)

Yeah this can be a match on is_truty

sokra · 2023-11-15T05:26:01Z

crates/turbopack-ecmascript/src/analyzer/mod.rs

@@ -1224,6 +1246,12 @@ impl JsValue {
                op.joiner(),
                b.explain_internal_inner(hints, indent_depth, depth, unknown_depth),
            ),
+            JsValue::Tenary(_, test, cons, alt) => format!(
+                "({}?{}:{})",


A bit more spacing here would be nice

sokra · 2023-11-15T05:28:34Z

crates/turbopack-ecmascript/tests/analyzer/graph/conditional-import/input.js

@@ -1,5 +1,7 @@
 const a = import(true ? "a" : "b")


This is testing your tenary support and should evaluate to a

I wonder how this already works before

Those tenary passes evaluation at

3c2d29c#diff-2c7634641094d97670b756458b73ab7bc5f6fba2da97471427a6c95795b8562dR429, while dynamic tenary evaluation doesn't seem to pass, so it goes to jsvalue:alternatives.

sokra · 2023-11-15T05:31:54Z

crates/turbopack-ecmascript/tests/analyzer/graph/conditional-import/resolved-effects.snapshot

@@ -3,10 +3,23 @@
 0 -> 2 call = import*0*("a")


Already seem to work?

github-actions · 2023-11-15T07:38:00Z

Linux Benchmark for `ce0ac01`

Click to view benchmark

Test	Base	PR	%
bench_hmr_to_commit/Turbopack CSR/1000 modules	21.75ms ± 0.86ms	21.91ms ± 0.67ms	+0.72%
bench_hmr_to_eval/Turbopack CSR/1000 modules	21.23ms ± 0.84ms	21.22ms ± 0.86ms	-0.05%
bench_startup/Turbopack CSR/1000 modules	1165.22ms ± 4.04ms	1142.94ms ± 11.23ms	-1.91%

github-actions · 2023-11-15T18:02:53Z

Linux Benchmark for `4ef8aa7`

Click to view benchmark

Test	Base	PR	%
bench_hmr_to_commit/Turbopack CSR/1000 modules	23.21ms ± 0.98ms	23.21ms ± 1.01ms	+0.02%
bench_hmr_to_eval/Turbopack CSR/1000 modules	22.62ms ± 0.95ms	22.62ms ± 0.97ms	+0.01%
bench_startup/Turbopack CSR/1000 modules	1160.77ms ± 5.82ms	1146.10ms ± 10.45ms	-1.26%

github-actions · 2023-11-16T23:26:37Z

Linux Benchmark for `94f2585`

Click to view benchmark

Test	Base	PR	%
bench_hmr_to_commit/Turbopack CSR/1000 modules	22.20ms ± 0.94ms	21.92ms ± 0.86ms	-1.28%
bench_hmr_to_eval/Turbopack CSR/1000 modules	21.66ms ± 0.78ms	21.54ms ± 0.89ms	-0.56%
bench_startup/Turbopack CSR/1000 modules	1155.48ms ± 6.75ms	1133.01ms ± 13.49ms	-1.94%

github-actions · 2023-11-17T00:38:17Z

Linux Benchmark for fb3cd66

Click to view benchmark

Test	Base	PR	%
bench_hmr_to_commit/Turbopack CSR/1000 modules	23.52ms ± 0.92ms	23.18ms ± 0.93ms	-1.44%
bench_hmr_to_eval/Turbopack CSR/1000 modules	22.52ms ± 0.99ms	23.19ms ± 1.12ms	+2.95%
bench_startup/Turbopack CSR/1000 modules	1157.48ms ± 6.38ms	1170.01ms ± 24.62ms	+1.08%

)" This reverts commit 2cf42a4.

…6496) Reverts #6452 Closes PACK-1981

Closes [PACK-1963](https://linear.app/vercel/issue/PACK-1963) Reattempt #6452 Closes PACK-1983

…borepo#6452)

…ercel/turborepo#6496) Reverts vercel/turborepo#6452 Closes PACK-1981

…borepo#6498) Closes [PACK-1963](https://linear.app/vercel/issue/PACK-1963) Reattempt vercel/turborepo#6452 Closes PACK-1983

…borepo#6452)

…ercel/turborepo#6496) Reverts vercel/turborepo#6452 Closes PACK-1981

…borepo#6498) Closes [PACK-1963](https://linear.app/vercel/issue/PACK-1963) Reattempt vercel/turborepo#6452 Closes PACK-1983

…borepo#6452)

…ercel/turborepo#6496) Reverts vercel/turborepo#6452 Closes PACK-1981

…borepo#6498) Closes [PACK-1963](https://linear.app/vercel/issue/PACK-1963) Reattempt vercel/turborepo#6452 Closes PACK-1983

kwonoj requested a review from a team as a code owner November 14, 2023 23:54

turbo-orchestrator bot added created-by: turbopack labels Nov 14, 2023

jridgewell approved these changes Nov 15, 2023

View reviewed changes

sokra reviewed Nov 15, 2023

View reviewed changes

kwonoj force-pushed the jsvalue-tenary branch from 9d1955d to 408e057 Compare November 15, 2023 07:21

kwonoj force-pushed the jsvalue-tenary branch from 408e057 to b7a889f Compare November 15, 2023 17:43

kwonoj force-pushed the jsvalue-tenary branch from b7a889f to ec280a4 Compare November 16, 2023 23:07

kwonoj added 2 commits November 16, 2023 16:20

feat(ecmascript): support dynamic tenary operator jsvalue

b19e8ac

test(ecmascipt): update snapshot

c78bd41

kwonoj force-pushed the jsvalue-tenary branch from ec280a4 to c78bd41 Compare November 17, 2023 00:21

kwonoj merged commit 2cf42a4 into main Nov 17, 2023
47 of 49 checks passed

kwonoj deleted the jsvalue-tenary branch November 17, 2023 00:44

sokra added a commit that referenced this pull request Nov 17, 2023

Revert "feat(ecmascript): support dynamic tenary operator jsvalue (#6452

59667f9

)" This reverts commit 2cf42a4.

sokra mentioned this pull request Nov 17, 2023

Revert "feat(ecmascript): support dynamic tenary operator jsvalue" #6496

Merged

sokra added a commit that referenced this pull request Nov 17, 2023

Revert "feat(ecmascript): support dynamic tenary operator jsvalue" (#…

2ed8790

…6496) Reverts #6452 Closes PACK-1981

kwonoj mentioned this pull request Nov 17, 2023

feat(ecmascript): support dynamic tenary operator jsvalue #6498

Merged

wbinnssmith pushed a commit that referenced this pull request Nov 17, 2023

feat(ecmascript): support dynamic tenary operator jsvalue (#6498)

c16e402

Closes [PACK-1963](https://linear.app/vercel/issue/PACK-1963) Reattempt #6452 Closes PACK-1983

ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024

feat(ecmascript): support dynamic tenary operator jsvalue (vercel/tur…

d37aeae

…borepo#6452)

ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024

Revert "feat(ecmascript): support dynamic tenary operator jsvalue" (v…

921c582

…ercel/turborepo#6496) Reverts vercel/turborepo#6452 Closes PACK-1981

ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024

feat(ecmascript): support dynamic tenary operator jsvalue (vercel/tur…

5a7dffd

…borepo#6452)

ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024

Revert "feat(ecmascript): support dynamic tenary operator jsvalue" (v…

8df0f2a

…ercel/turborepo#6496) Reverts vercel/turborepo#6452 Closes PACK-1981

ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024

feat(ecmascript): support dynamic tenary operator jsvalue (vercel/tur…

c72d1ea

…borepo#6452)

ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024

Revert "feat(ecmascript): support dynamic tenary operator jsvalue" (v…

c4e1fad

…ercel/turborepo#6496) Reverts vercel/turborepo#6452 Closes PACK-1981

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ecmascript): support dynamic tenary operator jsvalue #6452

feat(ecmascript): support dynamic tenary operator jsvalue #6452

kwonoj commented Nov 14, 2023 •

edited by padmaia

Loading

vercel bot commented Nov 14, 2023 •

edited

Loading

github-actions bot commented Nov 14, 2023 •

edited

Loading

github-actions bot commented Nov 15, 2023 •

edited

Loading

github-actions bot commented Nov 15, 2023

github-actions bot commented Nov 15, 2023 •

edited

Loading

jridgewell Nov 15, 2023

kwonoj Nov 15, 2023

sokra Nov 15, 2023

sokra Nov 15, 2023

sokra Nov 15, 2023

sokra Nov 15, 2023

kwonoj Nov 15, 2023

sokra Nov 15, 2023

github-actions bot commented Nov 15, 2023

github-actions bot commented Nov 15, 2023

github-actions bot commented Nov 16, 2023

github-actions bot commented Nov 17, 2023

feat(ecmascript): support dynamic tenary operator jsvalue #6452

feat(ecmascript): support dynamic tenary operator jsvalue #6452

Conversation

kwonoj commented Nov 14, 2023 • edited by padmaia Loading

Description

vercel bot commented Nov 14, 2023 • edited Loading

github-actions bot commented Nov 14, 2023 • edited Loading

github-actions bot commented Nov 15, 2023 • edited Loading

🟢 CI successful 🟢

github-actions bot commented Nov 15, 2023

Linux Benchmark for e99af59

github-actions bot commented Nov 15, 2023 • edited Loading

⚠️ Turbopack Benchmark CI failed ⚠️

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 15, 2023

Linux Benchmark for ce0ac01

github-actions bot commented Nov 15, 2023

Linux Benchmark for 4ef8aa7

github-actions bot commented Nov 16, 2023

Linux Benchmark for 94f2585

github-actions bot commented Nov 17, 2023

Linux Benchmark for fb3cd66

kwonoj commented Nov 14, 2023 •

edited by padmaia

Loading

vercel bot commented Nov 14, 2023 •

edited

Loading

github-actions bot commented Nov 14, 2023 •

edited

Loading

github-actions bot commented Nov 15, 2023 •

edited

Loading

Linux Benchmark for `e99af59`

github-actions bot commented Nov 15, 2023 •

edited

Loading

Linux Benchmark for `ce0ac01`

Linux Benchmark for `4ef8aa7`

Linux Benchmark for `94f2585`